[SPARK-57544][SQL] Rework column ID validation for nested fields in DSv2 by aokolnychyi · Pull Request #56619 · apache/spark

aokolnychyi · 2026-06-19T18:40:18Z

What changes were proposed in this pull request?

This PR reworks column ID validation for nested fields in DSv2.

Why are the changes needed?

The original implementation detected dropped-and-re-added columns by comparing top-level Column.id() strings in a dedicated validateColumnIds pass, but this approach had no visibility into nested struct fields, array elements, or map keys/values. To work around this limitation, connectors had to encode nested field IDs into the top-level ID string (as demonstrated by ComposedColumnIdTableCatalog), placing an unreasonable burden on connector authors and making the feature fragile by design.

The new mechanism stores field IDs in StructField metadata and validates within validateSchemaCompatibility.

Does this PR introduce any user-facing change?

Yes but it targets unreleased functionality and must be cherry picked to 4.2.

How was this patch tested?

Existing and new tests.

Was this patch authored or co-authored using generative AI tooling?

Claude Code v2.1.183

aokolnychyi · 2026-06-19T18:40:41Z

This is related to #55376.

aokolnychyi · 2026-06-19T18:45:29Z

@gengliangwang @juliuszsompolski @andreaschat-db @longvu-db, what do you folks think?

aokolnychyi · 2026-06-19T20:03:49Z

@huaxingao, this is the PR I was talking about on the dev list.

aokolnychyi · 2026-06-19T20:40:13Z

This is not yet complete as it doesn't handle stripping column IDs on CTAS/RTAS.

aokolnychyi · 2026-06-20T01:12:31Z

+        .build();
  }

-  /**


Switching newly added create methods to a builder. Hasn't been released yet.

aokolnychyi · 2026-06-20T01:14:12Z

+   * @param name the name of the column
+   * @param dataType the data type of the column
+   * @return a new builder
+   * @since 4.2.0


Intentional to be cherry-picked into 4.2.

aokolnychyi · 2026-06-20T01:18:57Z

+      return this;
+    }
+
+    public Builder metadataInJSON(String metadataInJSON) {


Probably call it metadata.

Renamed the method name, intentionally kept the variable name.

gengliangwang

2 blocking, 2 non-blocking, 1 nit.
Clean recursive generalization of the column-ID check, but two blockers: leftover references to the removed v1 machinery will fail tests, and the field-ID error message names the wrong (or no) field.

Design / architecture (2)

Incomplete removal of the column-ID-v1 machinery: DSv2IncrementallyConstructedQueryTests (run by the modified DataSourceV2DataFrameSuite and by DataSourceV2DataFrameConnectSuite) still asserts the removed INCOMPATIBLE_TABLE_CHANGE_AFTER_ANALYSIS.COLUMN_ID_MISMATCH at lines 324/414/488 — the path now throws COLUMNS_MISMATCH, so those checkErrors fail. ComposedColumnIdTableCatalog (now unreferenced anywhere; it is the workaround this PR eliminates) and NullTableIdAndNullColumnIdInMemoryTableCatalog still cite the removed validateColumnIds in comments. Migrate the trait's assertions to COLUMNS_MISMATCH and delete ComposedColumnIdTableCatalog. (likely overlaps the known "not yet complete" work)
CatalogV2Util.scala:676: FIELD_ID may ride into user-facing df.schema metadata — see inline (question).

Correctness (2)

SchemaUtils.scala:453: field-ID error names the parent (or empty) path, not the field whose ID changed — masked by tests asserting the wrong output — see inline.
SchemaUtils.scala:544: removeFieldIds struct branch always reallocates, contradicting its own Scaladoc — see inline.

Nits: 1 minor item (see inline comments).

PR description suggestions

Document: the new mechanism (field IDs stored per-field in StructField metadata and validated within validateSchemaCompatibility), not only the old approach's limitation — the "What changes" section is currently one line.
Document: the removal of the dedicated validateColumnIds pass and the COLUMN_ID_MISMATCH error subclass; field-ID changes now report as COLUMNS_MISMATCH.

gengliangwang · 2026-06-22T19:03:37Z

            case Some(otherField) =>
+              if (checkFieldIds) {
+                for (id <- field.id; otherId <- otherField.id if id != otherId) {
+                  errors += s"${colPath.fullyQuoted} field ID has changed from $id to $otherId"


This reports the parent struct's path, not the field whose ID changed. The check runs one level above the recursion that appends field.name, so unlike the sibling errors (formatField for removed/added, and the type/nullability messages emitted inside the recursion) it omits the field name: a top-level column change becomes " field ID has changed from 1 to 99" (empty path), and a nested change names the container — two changed sibling fields then produce indistinguishable messages.

Suggested change

errors += s"${colPath.fullyQuoted} field ID has changed from $id to $otherId"

errors += s"${(colPath :+ field.name).fullyQuoted} field ID has changed from $id to $otherId"

The six V2TableUtilSuite expectations (lines 569/587/602/621-622/639/656) assert the current output and will need updating; the explanatory comment at line 586 also carries a non-ASCII em-dash — switch it to ASCII while you're there.

gengliangwang · 2026-06-22T19:03:37Z

+   */
+  def removeFieldIds(dataType: DataType): DataType = dataType match {
+    case s: StructType =>
+      StructType(s.fields.map { field =>


This branch always rebuilds via StructType(s.fields.map(...)), so it never returns the original s — but the Scaladoc promises "the original instance unchanged when there is nothing to strip," which only the ArrayType/MapType branches honor. InMemoryBaseTable.assignFieldIds (line 945) shows the struct short-circuit: track whether any field changed and return s if none did. Either apply that here or relax the comment.

I relaxed the comment.

gengliangwang · 2026-06-22T19:03:37Z

      f = encodeDefaultValue(default, f)
    }
+    Option(col.id()).foreach { id =>
+      f = f.withId(id)


Encoding the top-level ID here also rides into the user-facing schema: DataSourceV2Relation.create builds its output from table.columns.asSchema (which calls this) via toAttributes, and FIELD_ID isn't in INTERNAL_METADATA_KEYS, so it looks like it ends up in df.schema field metadata for every column / nested field of an ID-tracking connector. Is that intended? If not, adding FIELD_ID to INTERNAL_METADATA_KEYS (or stripping it at the relation boundary) would keep it internal. I traced the encode path but didn't confirm end-user visibility — could you check?

Replied here.

gengliangwang · 2026-06-22T19:03:37Z

-   * IDs are preserved across type changes, keeping the same column ID through type
-   * widening and nested field additions. [[TypeChangeResetsColIdTableCatalog]] overrides
-   * this behavior for testing scenarios where type changes should produce a new ID.
+   * Existing IDs are preserved: Column → StructType → Column round-trip encodes them in


Non-ASCII arrows (U+2192); use ASCII (CLAUDE.md / scalastyle):

Suggested change

* Existing IDs are preserved: Column → StructType → Column round-trip encodes them in

* Existing IDs are preserved: Column -> StructType -> Column round-trip encodes them in

longvu-db · 2026-06-23T19:48:02Z

          "<errors>"
        ]
      },
-      "COLUMN_ID_MISMATCH" : {


Shall we have in-memory-table testings like in #55376 also where we test connector have implemented various scenarios?

What cases do you mean?

Like we have ComposedColumnIdTableCatalog, sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/TypeChangeResetsColIdTableCatalog.scala,.... where we mock various connector scenarios, e.g. Connector that provide top-level column ID only, no struct column ID; Connector that provides both top-level col ID and struct col ID,....

We replaced the approach in ComposedColumnIdTableCatalog, so I am not sure it is valid to keep. The rest of the tests should be in place.

longvu-db · 2026-06-23T19:51:09Z

    // semantics of SPARK-54444.
    val dataErrors = V2TableUtil.validateCapturedColumns(


We don't need to change validateCaputredColumns here validateLoadedTableInTransaction? This is part of the txn validation https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/V2TableReference.scala#L156

I think we are calling validateCapturedColumns with checkFieldIds: Boolean = true.
Is this enough?

Ah okay, makes sense

@aokolnychyi In that case, IMO it's clearer if we avoid setting a default value for checkFieldIds, so we force all current & future call-sites to be clear with the semantics

Yes and no, in my view. I removed some default but it makes sense to keep others as we should be validating IDs in most cases unless it is a temp view.

Removed a few more places for explicit usage.

aokolnychyi · 2026-06-24T06:00:44Z

+
+object FieldMetadataUtils {
+  // Metadata key for the field ID used to track column identity across schema evolution
+  val FIELD_ID_METADATA_KEY = "__FIELD_ID"


Went for __FIELD_ID instead of FIELD_ID to indicate a Spark internal key.

aokolnychyi · 2026-06-24T06:02:49Z

    AUTO_GENERATED_ALIAS,
    METADATA_COL_ATTR_KEY,
    QUALIFIED_ACCESS_ONLY,
+    FIELD_ID_METADATA_KEY,


Added here too. It is OK if this field surfaces in spark.table("t").schema but it should not appear in SHOW and has to be dropped in CTAS / RTAS kind of scenarios. Adding here ensures V1 tables creates from DSv2 tables don't inherit these extra columns.

Actually, let me think more about surfacing these keys in spark.table("t").schema.

It is OK, that's what we do for default values, for example.

aokolnychyi · 2026-06-24T06:05:12Z

      ref.info.columns,
-      mode = ALLOW_NEW_TOP_LEVEL_FIELDS)
+      mode = ALLOW_NEW_TOP_LEVEL_FIELDS,
+      checkFieldIds = false)


This seems reasonable for temp views.

gengliangwang

5 addressed, 0 remaining, 3 new (1 newly introduced, 2 late catches — my misses from earlier rounds).

2 blocking, 0 non-blocking, 1 nit. The rework is clean and all four prior findings plus the nit are resolved — but the "Address feedback" commit's Column.Builder rename left five callers stale, so the branch no longer compiles.

Correctness (2)

The branch doesn't compile: Column.Builder.metadataInJSON(...) was renamed to metadata(...), but five callers still use the old name — CatalogV2Util.scala:776/785/794/802 and ColumnSuite.scala:111. CI is red across all build modules. See inline.
TxnTable drops setVersion(delegate.version()) that master sets deliberately; with version-aware table equality this can break transaction re-resolution identity in tests — see inline (late catch).

Nits: 1 minor item — validateCapturedColumns / validateSchemaCompatibility Scaladoc omit the new field-ID check (see inline comments).

gengliangwang · 2026-06-24T21:44:32Z

+        .nullable(f.nullable)
+        .comment(comment)
+        .defaultValue(defaultValue)
+        .metadataInJSON(metadataAsJson(cleanedMetadata))


This doesn't compile. The builder method was renamed metadataInJSON(...) → metadata(...) in this commit (Column.java:270 now exposes only metadata(...)), but this call still uses the old name — as do 785, 794, 802 in this file and ColumnSuite.scala:111. CI is red across every build module (incl. api, catalyst and Precompile Spark).

Suggested change

.metadataInJSON(metadataAsJson(cleanedMetadata))

.metadata(metadataAsJson(cleanedMetadata))

Apply the same rename on lines 785, 794, 802, and ColumnSuite.scala:111.

gengliangwang · 2026-06-24T21:44:32Z

+  // Column IDs for existing columns are preserved through the StructType round-trip via
+  // metadata encoding. assignMissingIds assigns fresh IDs to any new columns added by
+  // schema evolution.
+  updateColumns(InMemoryBaseTable.assignMissingIds(columns()))


master sets setVersion(delegate.version()) right here ("the starting version should be the delegate version"), but this rewrite dropped it along with the old version comment. It looks unrelated to the column-ID change and reads as an accidental collateral deletion.

It matters: InMemoryBaseTable equality/hashCode are version-aware (id() == other.id() && version() == other.version()) and tableVersion defaults to 0, so the wrapper now reports version 0 instead of the delegate's — re-introducing the same identity false-positive the id override just above was added to prevent, now via version. Restore the line unless dropping it was intentional (in which case, what makes the version sync unnecessary now?).

gengliangwang · 2026-06-24T21:44:32Z

      originCols: Seq[Column],
-      mode: SchemaValidationMode = PROHIBIT_CHANGES): Seq[String] = {
+      mode: SchemaValidationMode = PROHIBIT_CHANGES,
+      checkFieldIds: Boolean = true): Seq[String] = {


The doc for this method lists type/nullability changes, removed, and added columns, but not the field-ID check it now performs — which is the headline behavior of this PR. Worth a line here and on SchemaUtils.validateSchemaCompatibility.

aokolnychyi · 2026-06-25T05:22:13Z

Updated and rebased. Ready for another round. Looked more into leaking, fixed CTAS / RTAS use cases.

longvu-db

Thank you very much for working on this Anton!

longvu-db · 2026-06-25T13:09:59Z

          "<errors>"
        ]
      },
-      "COLUMN_ID_MISMATCH" : {


Like we have ComposedColumnIdTableCatalog, sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/TypeChangeResetsColIdTableCatalog.scala,.... where we mock various connector scenarios, e.g. Connector that provide top-level column ID only, no struct column ID; Connector that provides both top-level col ID and struct col ID,....

longvu-db · 2026-06-25T13:16:40Z

+  /**
+   * Returns a copy of this field with the field ID removed, or this field if no ID is set.
+   */
+  def clearId(): StructField = {


Something tells me we should either return a copy in both scenarios, or in-place modification in both scenarios, for consistency.

P/s: if someone calls field.clearId(), it feels more intuitive as in-place modification for me, but I'm okay with this reutrning a copy

Also if someone does
a. fieldCopy = field.clearId() (where no ID is set) b. modification_to_the_fieldCopy`

Since currently, we return the field itself, would modification_to_the_fieldCopy modifies both the fieldCopy and field, sort of like indirect modification?

StructField is immutable by design and can't be modified.

longvu-db · 2026-06-25T14:04:04Z

+            "INTERNAL_ERROR",
+            Map.of("message",
+                "Column '" + name + "' cannot have more than one definition of: " +
+                "default value, generation expression, identity column spec"));


I assume in reality, this error shouldn't happen, so we cannot test it right?

Shouldn't happen in practice. We do test this in ColumnSuite, though. Just to be safe.

longvu-db · 2026-06-25T14:04:10Z

+    }
+
+    public Builder clearFieldIds() {
+      this.id = null;


Is it a problem if the clearId above returns a copy, but clearFieldIds here doesn't?

This is in the builder where we mutate the temp state before creating Column.

longvu-db · 2026-06-25T14:33:10Z

+
+      // Column IDs assigned by the catalog must NOT appear in SHOW CREATE TABLE output.
+      val showDDL = getShowCreateDDL(t)
+      assert(!showDDL.exists(_.contains("__FIELD_ID")), s"command must not expose column IDs")


Suggested change

assert(!showDDL.exists(_.contains("__FIELD_ID")), s"command must not expose column IDs")

assert(!showDDL.exists(_.contains(FIELD_ID_METADATA_KEY)), s"command must not expose column IDs")

longvu-db · 2026-06-25T14:34:30Z

-  // validation gets a chance to compare data types.
-  // Connect re-resolves both sides with the new column ID.
+  // The delete removes the old column and the add assigns a fresh one,
+  // so COLUMNS_MISMATCH fires in classic before schema validation gets a


schema validation is part of the COLUMNS_MISMATCH check, so I think it can be confusing to say COLUMNS_MISMATCH fires before schema validation, I think we should instead say "so column IDs check fires"

longvu-db · 2026-06-25T14:35:27Z

+  test("ReplaceTableAsSelect: field IDs in query schema are not propagated to table columns") {
+    val basicCatalog = catalog("testcat").asTableCatalog
+    val atomicCatalog = catalog("testcat_atomic").asTableCatalog
+    val basicIdentifier = "testcat.table_name"


Do we need to have withTable to clean up the identifier?

We do call withTable at line 674 or am I missing something?

nvm, my bad

longvu-db · 2026-06-25T14:38:46Z

-      classOf[ComposedColumnIdTableCatalog].getName)
-    .set("spark.sql.catalog.composedidcat.copyOnLoad", "true")
-    .set(SQLConf.TIME_TYPE_ENABLED.key, "true")
+    .set(InMemoryBaseTable.ASSIGN_COLUMN_IDS, "true")


I assume we need this flag ASSIGN_COLUMN_IDS because there are scenarios where we want to test table ID or schema name/data type validation?

Yeah, the idea to have this flag is to minimize the number of tests that have to be modified to account for column IDs that didn't exist before.

longvu-db · 2026-06-25T14:47:41Z


-  test("composed nested IDs tolerate nested field reorder end-to-end") {
-    val t = "composedidcat.ns1.ns2.tbl"
+  test("nested field reorder does not trigger column ID mismatch") {


// InMemoryTable does not actually reorder nested struct fields in stored
// data, so the read still returns the original field order. This is fine
// because the purpose of this test is to verify that the column ID check

Based on the comment, InMemoryTable does not actually do the re-ordering, it's a limitation

=> Let's try to fix InMemoryTable so that it actual does the re-ordering?

val nameParts = colPath :+ field.name
if (checkFieldIds) {
for (id <- field.id; otherId <- otherField.id if id != otherId) {
errors += s"${nameParts.fullyQuoted} field ID has changed from $id to $otherId"
}
}

Cuz the schema validation validates nested columns based on name, not position in the struct

=> I'm fine if this name matching mechanism is already tested in some way.

Removed this test and replaced with a more appropriate one. It will be tricky to implement reordering in the in-memory catalog.

longvu-db

LGTM apart from some comments

Thank you very much for working on this Anton!

longvu-db · 2026-06-25T21:03:33Z

      originCols: Seq[Column],
-      mode: SchemaValidationMode = PROHIBIT_CHANGES): Seq[String] = {
+      mode: SchemaValidationMode = PROHIBIT_CHANGES,
+      checkIds: Boolean = true): Seq[String] = {


Suggested change

checkIds: Boolean = true): Seq[String] = {

checkIds: Boolean): Seq[String] = {

longvu-db · 2026-06-25T21:03:40Z

      originMetaCols: Seq[MetadataColumn],
-      mode: SchemaValidationMode = PROHIBIT_CHANGES): Seq[String] = {
+      mode: SchemaValidationMode = PROHIBIT_CHANGES,
+      checkIds: Boolean = true): Seq[String] = {


Suggested change

checkIds: Boolean = true): Seq[String] = {

checkIds: Boolean): Seq[String] = {

longvu-db · 2026-06-25T21:05:07Z

+  /**
+   * Recursively clears field IDs from a data type.
+   */
+  def clearFieldIds(dataType: DataType): DataType = dataType match {


Nit: I wonder if we should call array elem, map key/value a "field" also

longvu-db · 2026-06-25T21:07:57Z

    val rawSchema = CharVarcharUtils.getRawSchema(removeInternalMetadata(schema), conf)
    val tableSchema = if (forceNullable) rawSchema.asNullable else rawSchema
-    CatalogV2Util.structTypeToV2Columns(tableSchema)
+    CatalogV2Util.structTypeToV2Columns(tableSchema, keepIds = false)


Suggested change

CatalogV2Util.structTypeToV2Columns(tableSchema, keepIds = false)

CatalogV2Util.structTypeToV2Columns(tableSchema, keepIds)

This won't compile, will it?

longvu-db · 2026-06-25T21:09:05Z

+          "tableName" -> ".*",
+          "errors" ->
+            """|
+               |- `salary` field ID has changed from \d+ to \d+


"salary field ID has changed from \d+ to \d+"

Does something look wrong here? i.e. the field ID didn't seem to change, and also it's a peculiar field ID name

I think it did change per comment above "reset-id catalog assigns a new ID for the widened column".

longvu-db · 2026-06-25T21:12:55Z

-import org.apache.spark.sql.connector.catalog.{CachingInMemoryTableCatalog, Column, ColumnDefaultValue, ComposedColumnIdTableCatalog, DefaultValue, GenerationExpression, Identifier, InMemoryTableCatalog, MixedColumnIdTableCatalog, NullColumnIdInMemoryTableCatalog, NullTableIdAndNullColumnIdInMemoryTableCatalog, NullTableIdInMemoryTableCatalog, SupportsV1OverwriteWithSaveAsTable, TableCatalog, TableInfo, TypeChangeResetsColIdTableCatalog}
-import org.apache.spark.sql.connector.catalog.BasicInMemoryTableCatalog
-import org.apache.spark.sql.connector.catalog.TableChange.{AddColumn, UpdateColumnDefaultValue}
+import org.apache.spark.sql.connector.catalog.{BasicInMemoryTableCatalog, CachingInMemoryTableCatalog, CatalogV2Util, Column, ColumnDefaultValue, DefaultValue, GenerationExpression, Identifier, InMemoryBaseTable, InMemoryTableCatalog, MixedColumnIdTableCatalog, NullColumnIdInMemoryTableCatalog, NullTableIdAndNullColumnIdInMemoryTableCatalog, NullTableIdInMemoryTableCatalog, SupportsV1OverwriteWithSaveAsTable, TableCatalog, TableInfo, TypeChangeResetsColIdTableCatalog}


Have we had a test where the Connector implements only the top-level Column ID but not the nested struct field ID? I assume sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/NullTableIdInMemoryTableCatalog.scala?

Not sure, maybe not, may need a follow-up PR.

Co-authored-by: Thang Long Vu <107926660+longvu-db@users.noreply.github.com>

gengliangwang · 2026-06-26T00:05:26Z

Thanks, merging to master/4.x
@aokolnychyi There are merge conflicts with 4.2. Can you create a separate cherry-pick for branch-4.2?

### What changes were proposed in this pull request? This PR reworks column ID validation for nested fields in DSv2. ### Why are the changes needed? The original implementation detected dropped-and-re-added columns by comparing top-level Column.id() strings in a dedicated validateColumnIds pass, but this approach had no visibility into nested struct fields, array elements, or map keys/values. To work around this limitation, connectors had to encode nested field IDs into the top-level ID string (as demonstrated by ComposedColumnIdTableCatalog), placing an unreasonable burden on connector authors and making the feature fragile by design. The new mechanism stores field IDs in `StructField` metadata and validates within `validateSchemaCompatibility`. ### Does this PR introduce _any_ user-facing change? Yes but it targets unreleased functionality and must be cherry picked to 4.2. ### How was this patch tested? Existing and new tests. ### Was this patch authored or co-authored using generative AI tooling? Claude Code v2.1.183 Closes #56619 from aokolnychyi/spark-57544. Authored-by: Anton Okolnychyi <aokolnychyi@apache.org> Signed-off-by: Gengliang Wang <gengliang@apache.org> (cherry picked from commit 64197c9) Signed-off-by: Gengliang Wang <gengliang@apache.org>

aokolnychyi · 2026-06-26T00:08:20Z

Let me look into creating a PR against 4.2. Thanks @gengliangwang @longvu-db!

aokolnychyi force-pushed the spark-57544 branch 2 times, most recently from 0b3f26f to 30864e4 Compare June 20, 2026 01:11

aokolnychyi commented Jun 20, 2026

View reviewed changes

aokolnychyi force-pushed the spark-57544 branch from 30864e4 to 1a82d94 Compare June 20, 2026 01:13

aokolnychyi commented Jun 20, 2026

View reviewed changes

gengliangwang reviewed Jun 22, 2026

View reviewed changes

longvu-db reviewed Jun 23, 2026

View reviewed changes

aokolnychyi commented Jun 24, 2026

View reviewed changes

gengliangwang reviewed Jun 24, 2026

View reviewed changes

aokolnychyi force-pushed the spark-57544 branch from 1f0f674 to 8ef82ea Compare June 25, 2026 05:18

longvu-db reviewed Jun 25, 2026

View reviewed changes

aokolnychyi force-pushed the spark-57544 branch from 36a512c to 3ffc7a0 Compare June 25, 2026 19:25

[SPARK-57544][SQL] Rework column ID validation for nested fields in DSv2

337f08b

aokolnychyi force-pushed the spark-57544 branch from 3ffc7a0 to 337f08b Compare June 25, 2026 19:27

gengliangwang approved these changes Jun 25, 2026

View reviewed changes

Fix useless test

5183a5e

longvu-db approved these changes Jun 25, 2026

View reviewed changes

aokolnychyi and others added 2 commits June 25, 2026 14:38

Remove some defaults

129971d

Apply suggestion from @longvu-db

b43c19a

Co-authored-by: Thang Long Vu <107926660+longvu-db@users.noreply.github.com>

gengliangwang closed this in 64197c9 Jun 26, 2026

	errors += s"${colPath.fullyQuoted} field ID has changed from $id to $otherId"
	errors += s"${(colPath :+ field.name).fullyQuoted} field ID has changed from $id to $otherId"

	* Existing IDs are preserved: Column → StructType → Column round-trip encodes them in
	* Existing IDs are preserved: Column -> StructType -> Column round-trip encodes them in

		// semantics of SPARK-54444.
		val dataErrors = V2TableUtil.validateCapturedColumns(

	.metadataInJSON(metadataAsJson(cleanedMetadata))
	.metadata(metadataAsJson(cleanedMetadata))

	assert(!showDDL.exists(_.contains("__FIELD_ID")), s"command must not expose column IDs")
	assert(!showDDL.exists(_.contains(FIELD_ID_METADATA_KEY)), s"command must not expose column IDs")

	checkIds: Boolean = true): Seq[String] = {
	checkIds: Boolean): Seq[String] = {

	CatalogV2Util.structTypeToV2Columns(tableSchema, keepIds = false)
	CatalogV2Util.structTypeToV2Columns(tableSchema, keepIds)

Uh oh!

Conversation

aokolnychyi commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

aokolnychyi commented Jun 19, 2026

Uh oh!

aokolnychyi commented Jun 19, 2026

Uh oh!

aokolnychyi commented Jun 19, 2026

Uh oh!

aokolnychyi commented Jun 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aokolnychyi Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gengliangwang left a comment

Choose a reason for hiding this comment

Design / architecture (2)

Correctness (2)

PR description suggestions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aokolnychyi commented Jun 19, 2026 •

edited

Loading

aokolnychyi Jun 24, 2026 •

edited

Loading